feat(sdk-core): add SdkWarmUp.prime() for CRaC auto-priming#7056
Conversation
| return; | ||
| } | ||
|
|
||
| ClasspathWarmUpInvoker.create().invokeAll(); |
There was a problem hiding this comment.
If this fails (raises some error), should we reset the PRIMED?
There was a problem hiding this comment.
Good call out.
Updated the logic to set primed only after invokeAll() succeeds , also made this part threadsafe.
| } | ||
|
|
||
| invokedAny = true; | ||
| try { |
There was a problem hiding this comment.
Is there a reason to have two seperate try catch blocks here?
There was a problem hiding this comment.
Each block catches a different failure.
The first block wraps iterator.next(), which is where a provider is located and instantiated. Per the ServiceLoader javadoc, this can throw ServiceConfigurationError if a provider-configuration file violates the specified format, if it names a provider class that cannot be found and instantiated, or if the result is not assignable to the service type. We catch that, log it, and continue with the remaining providers, so one bad provider does not abort the rest. Wrapping next() alone, not in a wider RuntimeException catch, keeps any other failure from next() visible.
The second block wraps provider.warmUp() and catches RuntimeException thrown by the provider's own code. We contain that one provider and still run the rest.
We intentionally contain failures from next() and warmUp() only. hasNext() stays in the loop condition, so if it throws we let it propagate rather than swallow it.
| return ServiceLoader.load(SdkWarmUpProvider.class, classLoader).iterator(); | ||
| } | ||
|
|
||
| private static final class CountingProvider implements SdkWarmUpProvider { |
There was a problem hiding this comment.
I'm a little confused about why we have effectively 3 test implementations that do the same thing (CoutningProvider, RealProvider and CountingWarmUpProvider)? And why does this one use a private invocation counter vs the other ones public constant?
There was a problem hiding this comment.
These got added as I built out the test scenarios. You're right that two of them did the same thing, so I removed the duplicate (RealProvider) and reused the existing one. I also renamed the remaining two to say how each is supplied:
CountingWarmUpProvider(old) ->RegisteredWarmUpProvider(new): registered inMETA-INF/servicesand discovered by a real ServiceLoader (so it's public with a no-arg ctor). Counter is static because ServiceLoader builds the instance and the test can't hold it, and public soClasspathWarmUpInvokerTest(a different package) can read it where it test for a failed and succesful Warmuploader.CountingProvider: a local stub handed straight to the invoker (no ServiceLoader), so it keeps a private instance counter.
…onventionWithSuppressionTest
joviegas
merged commit 98d7f72
into
feature/master/crac_auto_priming_support
|
This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one. |
2 participants
Motivation and Context
Next building block of CRaC (Coordinated Restore at Checkpoint) auto-priming: the warm-up entry point that drives priming before a checkpoint. Builds on the previously added
SdkWarmUpProviderSPI. Follows the same ServiceLoader discovery pattern the SDK already uses for SdkHttpService (the HTTP-client loader).Modifications
SdkWarmUpinsoftware.amazon.awssdk.core.crac: a@SdkPublicApifinal utility with a single staticprime(). It discovers everySdkWarmUpProvideron the classpath viaServiceLoaderand invokeswarmUp()on each. Modeled on the HTTP-client ServiceLoader loader pattern.AtomicBooleanrun-once guard.software.amazon.awssdk.core.internal.crac(WarmUpInvoker+ClasspathWarmUpInvoker+WarmUpServiceLoader), splitting the discover-and-invoke logic from the static entry point so it stays testable.License
License